Removed Redundant RwLock and Arc; Removed parking_lot Dependency
#75
Conversation
`TypeRegistryInternal` only needs to be available in an `Arc<RwLock<...>>` for Bevy scenes loading/unloading, which I don't believe is used at all for this plugin.
|
Is this a performance problem in practice (root of all-evil and all that)? Doesn't the |
To my knowledge, no, and I'd be surprised if any benchmark could even reliably measure any difference. My goal is just to simplify that part, since the
Not in this case. The reason for |
There was a problem hiding this comment.
Not in this case. The reason for
TypeRegistryandTypeRegistryInternalis that because of how scenes and assets are loaded, theTypeRegistryneeds to be accessed in a way that's only possible throughArc<RwLock<...>>. Forbevy_ggrs, that requirement does not exist. If you look at the definition forTypeRegistryin Bevy, you'll see a comment explaining the eventual desire to remove it entirely in favour ofTypeRegistryInternal. You can even see it through the type names, asTypeRegistryInternalis actually namedTypeRegistry, whileTypeRegistryis namedTypeRegistryArc.
Excellent. In that case, I have no objections. And nice with simpler code :)
Objective
The
RollbackTypeRegistrycontains aTypeRegistry, which itself is a thin wrapper aroundTypeRegistryInternal, but through anArc<RwLock<...>>. This indirection adds overhead to every interaction with the rollback type registry for no particular reason and should be removed. As a bonus, removing it would remove the need for theparking_lotdependency (still in the graph because of Bevy)Solution
Stored
TypeRegistryInternalinside theRollbackTypeRegistryinstead and removed lock acquisition code.